-
Notifications
You must be signed in to change notification settings - Fork 427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modern Ruby + ActiveRecord, Update Travis Config #416
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few questions to make sure we get this right..
Also, any reason why the build is currently failing? Hadn't the most recent commit been passing?
Oh, is it just because you added Rails 7? |
As far as I can tell the build has been failing for at least 4 years now, as shown here https://app.travis-ci.com/github/attr-encrypted/attr_encrypted/builds The last successful, per Travis, was this commit 11df93a. I am happy to start fixing things to get some of the builds passing, but my hunch says small changes here for historical context is better. |
I haven't added rails 7 to this yet, as it will fail too, but I can if you think. My thought was to add it in a separate PR where I can also try and get some of the existing tests to pass. |
@joshbranham, yes, but the sole commit to
If we can retrace our steps easily (as you point out, this isn't a big commit) re: the changes and thereby keep things passing, I think that's preferable. That said, I'm not opposed to merging if that's not possible (or if you think doing so would improve our work(flow) in some way). Lmk what you think. |
4183e62
to
e13c36d
Compare
@mvastola this has been updated as requested, we are now testing modern Ruby and ActiveRecord and passing with existing functionality. The only things I removed were the old versions, as discussed, and conditionals in the code that were checking old versions. Additionally, ActiveRecord 5.2 support was broken. A contributor added support in their fork to resolve this priyankatapar@7e8702b I think it might be worth reaching out to them to see if they want to open a pull adding that, either to master or my branch. What are your thoughts? It has been 3 years however. |
e13c36d
to
937b7e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is PERFECT! Thanks!
* Update Travis configuration * Drop os, use default * Use focal for OS * Restore patch versions of ruby, rvm not pulling latest patch * Update local test/run.sh script for easier local testing * Add some old AR back, remove 3+ rubies, drop deprecated codeclimate gem * Lock sqlite3 for Ruby 2.5 * ActiveRecord 6.0 and 6.1 tests failing, add later * Drop 2.5 * Update gemspec, pry for dev support * 5.1.1 AR works, update test/run.sh * Fix support for ActiveRecord 5.2
* Update Travis configuration * Drop os, use default * Use focal for OS * Restore patch versions of ruby, rvm not pulling latest patch * Update local test/run.sh script for easier local testing * Add some old AR back, remove 3+ rubies, drop deprecated codeclimate gem * Lock sqlite3 for Ruby 2.5 * ActiveRecord 6.0 and 6.1 tests failing, add later * Drop 2.5 * Update gemspec, pry for dev support * 5.1.1 AR works, update test/run.sh * Fix support for ActiveRecord 5.2
* Update Travis configuration * Drop os, use default * Use focal for OS * Restore patch versions of ruby, rvm not pulling latest patch * Update local test/run.sh script for easier local testing * Add some old AR back, remove 3+ rubies, drop deprecated codeclimate gem * Lock sqlite3 for Ruby 2.5 * ActiveRecord 6.0 and 6.1 tests failing, add later * Drop 2.5 * Update gemspec, pry for dev support * 5.1.1 AR works, update test/run.sh * Fix support for ActiveRecord 5.2
In an effort to make forward progress, and keep Travis credits from getting burned up too quickly, this modifies the configuration to:
With the above tests are now passing and this is ready for review so follow on work like adding Rails 7 support can continue.